Added Windows Support #259

Ginja · 2015-12-30T02:13:08Z

Fixes Windows install on 64-bit fails #236
Added Windows Support (binary install_method only) using NSSM
to manage the service. NSSM runs consul as the local SYSTEM user.
Support for running it as another user can/should be added later.
Prevented the firewall cookbook from creating rules for disabled
ports
Added & Updated spec tests
Disabled Style/ModuleFunction cop
Simplified how attributes with paths for values are set
Added version check to windows binary installation so that it can
be upgraded

codecov-io · 2015-12-30T02:42:32Z

Current coverage is `55.20%`

Merging #259 into master will increase coverage by +6.57% as of 5fa290d

@@            master   #259   diff @@
=====================================
  Files            6      7     +1
  Stmts          329    317    -12
  Branches         0      0       
  Methods          0      0       
=====================================
+ Hit            160    175    +15
  Partial          0      0       
+ Missed         169    142    -27

Review entire Coverage Diff as of 5fa290d

Powered by Codecov. Updated on successful CI builds.

attributes/default.rb

-default['consul']['config']['data_dir'] = '/var/lib/consul'
-default['consul']['config']['ca_file'] = '/etc/consul/ssl/CA/ca.crt'
-default['consul']['config']['cert_file'] = '/etc/consul/ssl/certs/consul.crt'
+case node['os']


Ginja · 2016-01-04T17:46:56Z

Rebased this PR to resolve conflicts.

gdavison · 2016-01-04T19:39:38Z

@Ginja, this looks great. One problem I've found is that building all the file paths with prefix_path puts some things in the wrong place on Linux. For example, the Consul data should be under the /var/lib/consul hierarchy, rather than /etc/consul

Ginja · 2016-01-04T20:43:33Z

@gdavison you're right, and there was brief discussion to change it here.

In order to keep everything clean & standardized on both platforms (i.e. consul data lives within the data folder underneath the prefix folder), I changed the values. I'd like to see it stay, but I can simply enforce that for my environment in our wrapper cookbook.

kamaradclimber · 2016-01-13T13:01:18Z

@johnbellone how can we help this PR getting merged?

gdavison · 2016-01-16T08:34:13Z

+1 on merging. It works great on our Windows boxes

johnbellone · 2016-01-19T16:58:58Z

Would you mind re-basing again?

attributes/default.rb

@@ -4,18 +4,23 @@
 #
 # Copyright 2014-2016, Bloomberg Finance L.P.
 #
+::Chef::Node.send(:include, ConsulCookbook::Helpers)


* Fixes #236 * Added Windows Support (binary install_method only) using NSSM to manage the service. NSSM runs consul as the local SYSTEM user. Support for running it as another user can/should be added later. * Prevented the firewall cookbook from creating rules for disabled ports * Added & Updated spec tests * Disabled Style/ModuleFunction cop

* Simplified how attributes with paths for values are set * Changed default values for data_dir attribute to keep everything simple * Added version check to windows binary installation so that it can be upgraded

@gdavison

Added a Windows only attribute that allows cookbook users to override and/or add their own nssm parameters. Added logic for comparing nssm parameters as the nssm resource doesn't currently do this. Reverted the change to the default path for the data directory. Borrowed some of these ideas from @gdavison, thanks!

Added exit code 1 as an acceptable return code within `nssm_service_installed?`. If AppRotateOnline is not set for nssm, it will only rotate the logs when the service is restarted. Changed the default nssm parameters so that logs are rotated when they reach 20 MB in size.

gdavison · 2016-01-19T22:16:27Z

One issue: consul_definition resources have to have their path set manually on Windows. (And the Linux version doesn't use the config_dir parameter, but that's a separate issue).

Not a show-stopper, though

Ginja · 2016-01-20T02:11:27Z

I've got a PR incoming that'll fix that. Thanks for the head's up. I just noticed the nssm resource doesn't start services if they're stopped. So I added that check into the Windows provider. We should look at contributing some of these changes back to the nssm cookbook.

Added a check to start Consul via nssm if it's not running. And updated consul_definition resource/provider with the new helper methods.

Added Windows Support

legal90 · 2016-01-29T10:11:46Z

Be aware - this PR introduces a backward incompatibility:

# Previous default value
default['consul']['config']['data_dir'] = '/var/lib/consul'

# New default value
default['consul']['config']['data_dir'] = join_path data_prefix_path, 'data'
# e.q. '/var/lib/consul/data'

cc: @johnbellone

lock · 2020-04-25T09:12:41Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Ginja mentioned this pull request Dec 30, 2015

Windows install on 64-bit fails #236

Closed

johnbellone reviewed Dec 30, 2015
View reviewed changes

johnbellone added the enhancement label Jan 19, 2016

johnbellone added this to the 1.4 milestone Jan 19, 2016

johnbellone reviewed Jan 19, 2016
View reviewed changes

attributes/default.rb

@@ -4,18 +4,23 @@

#

# Copyright 2014-2016, Bloomberg Finance L.P.

#

::Chef::Node.send(:include, ConsulCookbook::Helpers)

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

Ginja added 4 commits January 19, 2016 09:03

Simplified the setting of paths

8b18fbe

* Simplified how attributes with paths for values are set * Changed default values for data_dir attribute to keep everything simple * Added version check to windows binary installation so that it can be upgraded

Added nssm running check & updated consul_definition

cf8a9fb

Added a check to start Consul via nssm if it's not running. And updated consul_definition resource/provider with the new helper methods.

johnbellone added a commit that referenced this pull request Jan 20, 2016

Merge pull request #259 from visioncritical/windows-support

f3d3df5

Added Windows Support

johnbellone merged commit f3d3df5 into sous-chefs:master Jan 20, 2016

legal90 mentioned this pull request Jan 29, 2016

Remove duplicated "data_dir" attribute #269

Merged

Ginja mentioned this pull request Feb 5, 2016

Windows Consul service does not start up #273

Closed

knightorc deleted the windows-support branch June 6, 2018 23:05

lock bot locked as resolved and limited conversation to collaborators Apr 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Windows Support #259

Added Windows Support #259

Ginja commented Dec 30, 2015

codecov-io commented Dec 30, 2015

This comment was marked as outdated.

This comment was marked as outdated.

Ginja commented Jan 4, 2016

gdavison commented Jan 4, 2016

Ginja commented Jan 4, 2016

kamaradclimber commented Jan 13, 2016

gdavison commented Jan 16, 2016

johnbellone commented Jan 19, 2016

This comment was marked as outdated.

This comment was marked as outdated.

gdavison commented Jan 19, 2016

Ginja commented Jan 20, 2016

legal90 commented Jan 29, 2016

lock bot commented Apr 25, 2020

Added Windows Support #259

Added Windows Support #259

Conversation

Ginja commented Dec 30, 2015

codecov-io commented Dec 30, 2015

Current coverage is 55.20%

This comment was marked as outdated.

This comment was marked as outdated.

Ginja commented Jan 4, 2016

gdavison commented Jan 4, 2016

Ginja commented Jan 4, 2016

kamaradclimber commented Jan 13, 2016

gdavison commented Jan 16, 2016

johnbellone commented Jan 19, 2016

This comment was marked as outdated.

This comment was marked as outdated.

gdavison commented Jan 19, 2016

Ginja commented Jan 20, 2016

legal90 commented Jan 29, 2016

lock bot commented Apr 25, 2020

Current coverage is `55.20%`